Skip to content

Conversation

@kusalk
Copy link
Member

@kusalk kusalk commented Jan 3, 2024

WW-5352

General clean up in preparation for #832

@kusalk kusalk force-pushed the WW-5352-parameter-annotation-2 branch from 9afea18 to 5c7a6bc Compare January 3, 2024 10:22
@kusalk kusalk force-pushed the WW-5352-parameter-annotation-2 branch from 5c7a6bc to 8a245e8 Compare January 3, 2024 10:34
Base automatically changed from WW-5352-parameter-annotation to master January 3, 2024 12:04
@kusalk kusalk marked this pull request as ready for review January 3, 2024 12:08
* @param value to check
* @return object containing result of matched pattern and pattern itself
*/
public IsAccepted isAccepted(String value);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant in interfaces

@Override
public String getValue() {
String[] values = toStringArray();
return (values != null && values.length > 0) ? values[0] : null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#toStringArray never returns a null value

};

@Override
public String doIntercept(ActionInvocation invocation) throws Exception {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used early returns to reduce nesting and improve readability

protected void addParametersToContext(ActionContext ac, Map<String, ?> newParams) {
}

protected void setParameters(final Object action, ValueStack stack, HttpParameters parameters) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted a bunch of helper methods to make this method much more readable

boolean acceptableParamValue = (parameterValueAware == null || parameterValueAware.acceptableParameterValue(param.getValue()));
if (hasParamValuesToExclude() || hasParamValuesToAccept()) {
// Additional validations to process
acceptableParamValue &= acceptableValue(param.getName(), param.getValue());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&= doesn't short-circuit

return false;
}
boolean accepted = isWithinLengthLimit(name) && !isExcluded(name) && isAccepted(name);
if (devMode && accepted) { // notify only when in devMode
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned out comments for behaviours which are fairly obvious from code

@kusalk kusalk requested a review from lukaszlenart January 4, 2024 00:57
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions

10 Security Hotspots
28.7% Coverage on New Code (required ≥ 80%)
4.1% Duplication on New Code (required ≤ 3%)
E Security Rating on New Code (required ≥ A)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@kusalk
Copy link
Member Author

kusalk commented Jan 5, 2024

@lukaszlenart Any idea how to configure a second base branch for SonarCloud dedicated to 7.x so we don't get wonky results?

@kusalk kusalk requested a review from lukaszlenart January 5, 2024 15:52
@kusalk kusalk merged commit ecd02de into master Jan 6, 2024
@kusalk kusalk deleted the WW-5352-parameter-annotation-2 branch January 6, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants